Skip to content

fix(react): resolve builtin elements from namespace-imported styled-components#110

Merged
async3619 merged 3 commits intodevfrom
issue-108
Mar 28, 2026
Merged

fix(react): resolve builtin elements from namespace-imported styled-components#110
async3619 merged 3 commits intodevfrom
issue-108

Conversation

@async3619
Copy link
Copy Markdown
Owner

Summary

  • Namespace import support: Record ImportNamespaceSpecifier bindings and handle JSXMemberExpression references (e.g. <Styled.Section>) so the analyzer follows namespace imports to resolve the underlying builtin HTML elements from styled-component factory calls in external files.
  • Diff snapshot perf: Replace the per-file git cat-file + fs.writeFileSync loop in materializeGitTreeSnapshot with a single git archive | tar streaming pipe, reducing diff snapshot time from ~2 min to ~6 s on a 4,140-file repository.

Related Issue

Closes #108

Validation

  • pnpm run check
  • Commits follow Conventional Commits
  • Branch name follows codex/issue-<number>-<slug>
  • This PR will be Squash Merged into dev

Notes

  • The git archive | tar approach uses shell positional parameters ($0, $1, $2) to avoid injection risks while enabling streaming without memory buffering.

async3619 and others added 2 commits March 28, 2026 16:30
…omponents

Record ImportNamespaceSpecifier bindings and handle JSXMemberExpression
references (e.g. `<Styled.Section>`) so the analyzer follows namespace
imports to resolve the underlying builtin HTML elements.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the N × git-cat-file + fs.writeFileSync loop in
materializeGitTreeSnapshot with a single `git archive | tar` pipe.
On a 4 140-file repository this reduces diff snapshot time from ~2 min
to ~2 s by eliminating thousands of process spawns and disk round-trips.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Support namespace-imported styled-components and optimize git snapshot performance

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Resolve builtin elements from namespace-imported styled-components
  - Record ImportNamespaceSpecifier bindings for namespace imports
  - Handle JSXMemberExpression references (e.g. <Styled.Section>)
  - Follow namespace imports to resolve underlying HTML elements
• Replace per-file git operations with streaming git archive
  - Eliminate thousands of process spawns and disk round-trips
  - Reduce diff snapshot time from ~2 min to ~6 s on large repositories
Diagram
flowchart LR
  A["Namespace Import<br/>Styled.*"] -->|Record ImportNamespaceSpecifier| B["Bindings Storage"]
  B -->|Resolve JSXMemberExpression| C["Member Reference<br/>e.g. Styled.Section"]
  C -->|Follow namespace import| D["Source File Analysis"]
  D -->|Resolve exported symbol| E["Builtin Element<br/>e.g. section"]
  F["Git Tree Snapshot"] -->|Replace per-file<br/>cat-file| G["Streaming git archive<br/>pipe to tar"]
  G -->|Extract to temp dir| H["Materialized Snapshot"]
Loading

Grey Divider

File Changes

1. e2e/react.test.ts 🧪 Tests +71/-0

Add test for namespace-imported styled-components

e2e/react.test.ts


2. src/analyzers/react/bindings.ts ✨ Enhancement +9/-0

Record ImportNamespaceSpecifier bindings

src/analyzers/react/bindings.ts


3. src/analyzers/react/diff.ts Performance optimization +19/-36

Replace git cat-file with streaming git archive

src/analyzers/react/diff.ts


View more (6)
4. src/analyzers/react/entries.ts ✨ Enhancement +4/-1

Handle JSXMemberExpression component references

src/analyzers/react/entries.ts


5. src/analyzers/react/references.ts ✨ Enhancement +41/-0

Resolve namespace member references to exports

src/analyzers/react/references.ts


6. src/analyzers/react/usage.ts ✨ Enhancement +6/-0

Collect member expression component references

src/analyzers/react/usage.ts


7. src/analyzers/react/walk.ts ✨ Enhancement +22/-0

Extract member expression component reference names

src/analyzers/react/walk.ts


8. test/fixtures/react-mode/src/components/FeatureSection.styled.tsx 🧪 Tests +9/-0

Add styled-components fixture for namespace imports

test/fixtures/react-mode/src/components/FeatureSection.styled.tsx


9. test/fixtures/react-mode/src/namespace-styled-entry.tsx 🧪 Tests +9/-0

Add entry point fixture using namespace-imported styled

test/fixtures/react-mode/src/namespace-styled-entry.tsx


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Mar 28, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. Snapshot requires POSIX shell🐞 Bug ⛯ Reliability
Description
materializeGitTreeSnapshot now invokes sh -c 'git ... | tar ...', which will fail on systems
without a POSIX shell (commonly Windows) and makes foresthouse react --diff unusable there.
Code

src/analyzers/react/diff.ts[R993-1005]

+  const result = spawnSync(
+    'sh',
+    [
+      '-c',
+      'git -C "$0" archive "$1" | tar -xf - -C "$2"',
    repositoryRoot,
-      ['cat-file', '-p', `${tree}:${filePath}`],
-      {
-        trim: false,
-      },
-    )
-    const absolutePath = path.join(snapshotRoot, ...filePath.split('/'))
-
-    fs.writeFileSync(absolutePath, fileContent)
-  })
-
-  return snapshotRoot
-}
-
-function ensureSnapshotDirectory(snapshotRoot: string, filePath: string): void {
-  const parentDirectory = path.dirname(filePath)
+      tree,
+      snapshotRoot,
+    ],
+    {
+      stdio: ['ignore', 'pipe', 'pipe'],
+    },
+  )
Evidence
The snapshot extraction is executed via spawnSync('sh', ['-c', ...]), so the feature depends on
sh being present in PATH. The README positions this as a Node.js CLI and documents `foresthouse
react --diff` without stating Unix-only prerequisites.

src/analyzers/react/diff.ts[985-1005]
README.md[8-34]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`materializeGitTreeSnapshot()` currently shells out via `sh -c` to run a pipeline (`git archive | tar`). This breaks on environments without a POSIX shell (notably many Windows installs), causing `foresthouse react --diff` to fail immediately.
## Issue Context
The tool is a Node.js CLI and the README advertises `foresthouse react --diff` without platform constraints; relying on `sh` is a new, undocumented runtime requirement.
## Fix Focus Areas
- src/analyzers/react/diff.ts[985-1015]
## Suggested fix approach
- Avoid `sh -c` entirely.
- Prefer one of:
- **Two-process pipeline without a shell**: spawn `git` and `tar` directly (e.g., `spawn('git', ['-C', repo, 'archive', tree])` piped to `spawn('tar', ['-xf', '-', '-C', snapshotRoot])`), handling exit codes for both.
- **Portable fallback**: if `process.platform === 'win32'` or `tar` is unavailable, fall back to the prior per-file `git cat-file` materialization method.
- If you keep `tar`, consider checking for command availability up front and emitting a clear error explaining prerequisites.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Blank snapshot failure error🐞 Bug ✓ Correctness
Description
On snapshot extraction failure with empty stderr, the thrown error message can become blank because
?? treats empty string as a valid value, hiding the actual failure cause.
Code

src/analyzers/react/diff.ts[R1007-1011]

+  if (result.error !== undefined || result.status !== 0) {
+    fs.rmSync(snapshotRoot, { recursive: true, force: true })
+    throw new Error(
+      `Failed to extract Git tree snapshot: ${result.stderr?.toString('utf8').trim() ?? result.error?.message ?? 'Unknown error'}`,
+    )
Evidence
The error message uses result.stderr?.toString('utf8').trim() ?? ...; if stderr is an empty
string, it is not nullish so the fallback to result.error?.message / Unknown error won’t run,
producing Failed to extract Git tree snapshot:  with no details.

src/analyzers/react/diff.ts[1007-1011]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`materializeGitTreeSnapshot()` can throw an error with an empty/blank reason when the subprocess exits non-zero but writes nothing to stderr.
## Issue Context
The error string is built using nullish coalescing (`??`) on a potentially empty trimmed stderr string; empty string is not nullish, so the fallback message is skipped.
## Fix Focus Areas
- src/analyzers/react/diff.ts[1007-1011]
## Suggested fix approach
- Compute stderr as a trimmed string and only use it if it’s non-empty.
- Include additional context such as `status`/`signal`.
Example direction (not exact code):
- `const stderr = Buffer.isBuffer(result.stderr) ? result.stderr.toString('utf8').trim() : String(result.stderr ?? '').trim()`
- `const details = stderr.length > 0 ? stderr : result.error?.message ?? `exit ${result.status ?? 'unknown'}``
- Throw with `details`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Adopt dev's git archive + CoW clone implementation over our spawnSync
variant — both solved the same per-file cat-file bottleneck, dev's
version adds CoW snapshot optimization on top.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 28, 2026

Codecov Report

❌ Patch coverage is 60.71429% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.13%. Comparing base (b38bf71) to head (1ae5ce1).
⚠️ Report is 2 commits behind head on dev.

Files with missing lines Patch % Lines
src/analyzers/react/references.ts 63.63% 2 Missing and 2 partials ⚠️
src/analyzers/react/walk.ts 60.00% 2 Missing and 2 partials ⚠️
src/analyzers/react/bindings.ts 33.33% 0 Missing and 2 partials ⚠️
src/analyzers/react/entries.ts 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #110      +/-   ##
==========================================
+ Coverage   65.97%   66.13%   +0.15%     
==========================================
  Files          39       39              
  Lines        2613     2640      +27     
  Branches      862      873      +11     
==========================================
+ Hits         1724     1746      +22     
+ Misses        526      525       -1     
- Partials      363      369       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq bot commented Mar 28, 2026

Merging this PR will not alter performance

✅ 18 untouched benchmarks


Comparing issue-108 (1ae5ce1) with dev (b38bf71)

Open in CodSpeed

@async3619 async3619 merged commit 2a76067 into dev Mar 28, 2026
4 of 5 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 28, 2026
## [1.2.0-dev.1](v1.1.1-dev.1...v1.2.0-dev.1) (2026-03-28)

### Features

* **react:** add usage diff and benchmark coverage ([#107](#107)) ([4ae9e65](4ae9e65))

### Bug Fixes

* **react:** resolve builtin elements from namespace-imported styled-components ([#110](#110)) ([2a76067](2a76067))

### Performance

* **react:** optimize git tree materialization for diff ([#109](#109)) ([b38bf71](b38bf71))
@async3619
Copy link
Copy Markdown
Owner Author

🎉 This PR is included in version 1.2.0-dev.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

react: cannot resolve builtin elements from externally imported styled-components

1 participant